-
Notifications
You must be signed in to change notification settings - Fork 6.1k
Disallow non-default expectedExecutionsPerDeployment
in evmasm tests when only a creation object is present
#16023
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Disallow non-default expectedExecutionsPerDeployment
in evmasm tests when only a creation object is present
#16023
Conversation
12f8750
to
202933b
Compare
This pull request is stale because it has been open for 14 days with no activity. |
This pull request is stale because it has been open for 14 days with no activity. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cameel Needs a rebase which the github UI refuses to do, looks good to me otherwise. It is a useful error to have and can save headache.
This pull request is stale because it has been open for 14 days with no activity. |
Depends on #16021.
This adds an extra check to prevent confusion like in #16012 (comment).
expectedExecutionsPerDeployment
only affects runtime assemblies, but the top-level assembly is a creation one so if you have only one, it will seem like the setting is not working. This PR makes the test case throw an exception in that situation to make it obvious. It is not likely to be intentional.I did not include it in #16021, because I'm not yet 100% convinced we want this. There are some downsides:
--import-asm-json
would just accept it.Still, these seem minor enough that we're probably better off with this check than without it.